Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce mkBinaryCache function #194345

Merged
merged 2 commits into from
Feb 9, 2023
Merged

Conversation

thomasjm
Copy link
Contributor

@thomasjm thomasjm commented Oct 4, 2022

Description of changes

This introduces a new function mkBinaryCache which can be used to construct a flat-file binary cache (i.e. one you can use with the file:// prefix) from a derivation. It's inspired by the closureInfo function and uses the same techniques to construct .narinfo files.

My use-case that I'd like to test Nix builds in a containerized CI environment, without downloading a bunch of stuff on every pipeline run. Thus, I want to make a binary cache of some kind available inside the container. The most efficient way to do this is with a bind-mount.

Initially I tried just mounting /nix/store to a path on the container, but I soon ran into a problem: you can either mount read/write, in which case the container might potentially mess with the integrity of the store, especially when run as root. Or, you can mount read-only, in which case you run into NixOS/nix#6835. Some discussion on the Nix hackers Matrix channel showed that mounting the store read-only is unlikely to be solvable because of how the Sqlite DB works.

So, making a flat-file binary cache and then bind-mounting it seemed like the best approach. I initially tried just running nix copy inside a derivation but that didn't work well, so I came up with this function.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • [N/A] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • [N/A] Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Oct 4, 2022
@ghost
Copy link

ghost commented Oct 4, 2022

Some discussion on the Nix hackers Matrix channel showed that NixOS/nix#6835 is unlikely to be solvable because of how the Sqlite DB works.

What was the reasoning there? I've used sqlite on readonly-mounted devices before. You might have to disable locking, which means promising that nothing else has read-write access to the same database.

@thomasjm
Copy link
Contributor Author

thomasjm commented Oct 4, 2022

disable locking, which means promising that nothing else has read-write access to the same database

You're right that SQLite can operate in that mode, but it's not really viable when you're talking about mounting the main store of the CI machine which is busily building other things.

FWIW, I did come up with one (fragile and intricate) solution where you manually create a writable overlay filesystem on /nix/var/nix/db and mount that. This allows Nix to work with the DB normally, but probably still leads to undefined behavior if the host database changes unexpectedly.

There is probably also the possibility for weirdness because IIRC Nix sometimes creates store paths while performing apparently side-effect-free operations, which would not work on the read-only mount--I'm not sure if this is a possibility when the store is being used as a substituter.

Lastly I forgot to mention, but there were also other problems with read-only mounts, like the one I described in the linked issue. I spent a while trying to hack Nix to operate in a read-only mode where it wouldn't try to do things like chmod directories, but it was kind of a mess. (I misspoke when I conflated NixOS/nix#6835 with the SQLite DB issues--just edited for clarity.)

@ghost
Copy link

ghost commented Oct 4, 2022

I spent a while trying to hack Nix to operate in a read-only mode where it wouldn't try to do things like chmod directories, but it was kind of a mess.

I tried that too at one point and got pretty much the same results.

It is pretty weird how insistent Nix is about being able to write to /nix/store.

@thomasjm
Copy link
Contributor Author

thomasjm commented Oct 4, 2022

Indeed haha our beautiful pure functional abstraction is built on a thing that loves to randomly chmod stuff.

Any chance you'd be willing to review this?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I guess I'm a bit unclear on the use case for this being in nixpkgs... you've put it into build-support, but nothing in nixpkgs uses it in order to build any packages. Is there some future package, to be in nixpkgs, which needs this in order to support its build? The use case mentioned at the top of the PR talks about a CI system, but presumably that is site-specific to your organization, right?

I'm also a bit unsure about the use of recursive nix (although it is not IFD/EBEB) and the assumptions about the (AFAICT undocumented/unspecified) structure of narinfo files.

pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
pkgs/build-support/binary-cache.nix Outdated Show resolved Hide resolved
finalXzFile = "nar/" + fileHash + ".nar.xz"
os.rename(xzFile, finalXzFile)

with open(narInfoHash + ".narinfo", "w") as f:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there some way to get nix to create the narinfo?

AFAICT the file format isn't specified anywhere, so if it changes this won't get updated...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that I'm aware of, but I'd love to find one. It seems pretty ad-hoc, look at this CGI script in nix-binary-cache:

@ghost
Copy link

ghost commented Oct 4, 2022

I initially tried just running nix copy inside a derivation but that didn't work well,

What was the failure?

If nix copy can be made to work inside a derivation I think that's a better approach. This has to be recursive nix in either case, so it's no worse there. And on the upside, it won't duplicate the narinfo-file-creation code already in nix.

@thomasjm
Copy link
Contributor Author

thomasjm commented Oct 5, 2022

Overall I guess I'm a bit unclear on the use case for this being in nixpkgs

The argument is just that this is a useful function, for working with a "core" Nix functionality, which ought to be generally available.

I'm also a bit unsure about the use of recursive nix

I wouldn't call it recursive nix -- it uses nix-store --dump and nix-hash. The former creates a NAR file and the latter hashes it. These are "utility" functions which happen to reside inside nix-store and nix-hash executables, and could be replaced with other tools. There's certainly no derivation building going on here.

  • For nix-store --dump: I looked around for a simple standalone tool to make NAR archives, but couldn't find one.
  • For nix-hash: any sha256 hashing tool would work, but I wanted to create the non-standard sha256:XXX style hashes which Nix uses, to match the behavior of nix copy.

If nix copy can be made to work inside a derivation I think that's a better approach.

I found nix copy was trying to access the internet, even though I was just trying to do --to file://$out. Running with --impure didn't fix it, it seemed confused that the Nix DB wasn't available.

@ghost
Copy link

ghost commented Oct 5, 2022

The argument is just that this is a useful function

But is it useful to nixpkgs? Nixpkgs is (a) a collection of expressions for building software and (b) Nix subroutines needed by (a).

If there is some software you want to package within nixpkgs that needs this subroutine, it would be a great idea to include it with the PR which adds that software package.

@thomasjm
Copy link
Contributor Author

thomasjm commented Oct 7, 2022

But is it useful to Nixpkgs?

I would say definitely! A Nix binary cache is a perfectly valid kind of software that I might want to build. It is an end goal in itself, if you're trying to work with Nix substituters.

For a close analogy of something already in Nixpkgs, consider dockerTools. Nixpkgs only actually uses it in a few places, but exports it because it is useful for people to be able to build Docker containers--they too are a useful software end product.

Since Nix binary caches are a native part of the Nix ecosystem, Nixpkgs seems like a very natural place to add support.

@ajs124
Copy link
Member

ajs124 commented Oct 27, 2022

Without having read the whole thread: this has the same issues as closureInfo, as described in #123943 (review) and NixOS/nix#4840

@ghost
Copy link

ghost commented Oct 30, 2022

this has the same issues as closureInfo, as described in #123943 (review) and NixOS/nix#4840

I don't think so. This PR doesn't involve nix-store --load-db.

This PR doesn't try to separate the DB from the store paths and then restore the store paths using substituters. As you noted in #123943, that's what caused the problems.

@ghost
Copy link

ghost commented Oct 30, 2022

In fact, this PR does exactly what @roberth said all uses of closureInfo ought to do:

this functionality must only be used in derivations that produce no references to the original paths; instead incorporating all paths in an image (or similar)

That's what this PR does. And it is packaged in a way that makes it impossible to not do this.

So it does appear as if this is less footgun-prone than closureInfo. Unfortunately it is not really a replacement for closureInfo; they don't do the same thing, although there is some overlap in functionality.

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Needs a test; perhaps a NixOS VM test?
  • Needs documentation in the Nixpkgs manual.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 31, 2022
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Oct 31, 2022
@thomasjm
Copy link
Contributor Author

Okay, I've taken a stab at documentation.

I'm having a little trouble adding a NixOS test, because the VM environment seems not to having a working NIX_PATH with nixpkgs. I've found that NIX_PATH equals NIX_PATH=nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos:nixos-config=/etc/nixos/configuration.nix:/nix/var/nix/profiles/per-user/root/channels.

However, I'm finding that /nix/var/nix/profiles/per-user/root is an empty directory. Do you happen to know if there's some test setup I'm missing @roberth ?

@roberth
Copy link
Member

roberth commented Oct 31, 2022

A channel can be set up by importing nixos/modules/installer/cd-dvd/channel.nix

Suppose you want to build pkg, then you can add pkg.inputDerivation to the cache to provide the build dependencies.

@thomasjm
Copy link
Contributor Author

thomasjm commented Oct 31, 2022

That helped, thanks!

I'm having trouble with the inputDerivation thing though. I was hoping adding hello.inputDerivation to the systemPackages would result in a VM where the full closure of hello is present. But instead the test fails to build with error: The store path /nix/store/ab0dn2k2xhx91amhr5dab82k4fwr1q99-hello-2.12.1 is a file and can't be merged into an environment using pkgs.buildEnv! at /nix/store/0v81528s8264h402mzbchvc1k3m937xy-builder.pl line 122.. Any chance you can see what I'm doing wrong?

@roberth
Copy link
Member

roberth commented Oct 31, 2022

You could use system.extraDependencies, or you could add it to mkBinaryCache instead.

@thomasjm
Copy link
Contributor Author

Okay, got a working VM test!

@thomasjm thomasjm requested a review from roberth October 31, 2022 12:16
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Oct 31, 2022
@thomasjm
Copy link
Contributor Author

Friendly ping @roberth, I think this is ready to go!

@thomasjm
Copy link
Contributor Author

thomasjm commented Feb 1, 2023

@roberth just checking in again, think we can merge this? Thanks!

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. Could you try the suggestions? Otherwise lgtm!

pkgs/build-support/binary-cache/default.nix Show resolved Hide resolved
pkgs/build-support/binary-cache/default.nix Show resolved Hide resolved
@roberth
Copy link
Member

roberth commented Feb 3, 2023

nix shell nix will make you a nix that supports it, although you do have to set an experimental feature for now.

@roberth
Copy link
Member

roberth commented Feb 3, 2023

Oh, no, no, no, no, no. That's not right. You do need to update your system nix or use a relocated store "chroot" store.

nix shell nix
nix --store ~/experimental-store --extra-experimental-features discard-references build nixosTests.binary-cache  # etc

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 5, 2023
@thomasjm
Copy link
Contributor Author

thomasjm commented Feb 6, 2023

Would you be interested in trying it out and updating this PR to set the right flag?

Sure -- would that be unsafeDiscardReferences.out = true ?

I'm having some trouble applying your suggestions above. Both cause the tests to fail in slightly different ways. Error messages are below. I have no idea why these cause failures, maybe the code this is based on has to be just so. Help?

Suggestion 1: Use nativeBuildInputs/buildCommand instead of PATH/builder

machine # building '/nix/store/x30m3rvz7j39imcm6i25mf444kiavlbp-acl-2.3.1.tar.gz.drv'...
machine # warning: error: unable to download 'https://mirror.easyname.at/nongnu/acl/acl-2.3.1.tar.gz': Couldn't resolve host name (6); retrying in 288 ms
machine # warning: error: unable to download 'https://mirror.easyname.at/nongnu/acl/acl-2.3.1.tar.gz': Couldn't resolve host name (6); retrying in 603 ms
machine # warning: error: unable to download 'https://mirror.easyname.at/nongnu/acl/acl-2.3.1.tar.gz': Couldn't resolve host name (6); retrying in 1412 ms
machine # warning: error: unable to download 'https://mirror.easyname.at/nongnu/acl/acl-2.3.1.tar.gz': Couldn't resolve host name (6); retrying in 2016 ms
machine # error: unable to download 'https://mirror.easyname.at/nongnu/acl/acl-2.3.1.tar.gz': Couldn't resolve host name (6)
machine # error: builder for '/nix/store/x30m3rvz7j39imcm6i25mf444kiavlbp-acl-2.3.1.tar.gz.drv' failed with exit code 1
machine # error: 1 dependencies of derivation '/nix/store/1aq767kvk0v8s7z7rvbc6ymlnxrpf85f-acl-2.3.1.drv' failed to build
machine # building '/nix/store/c99ihlhb2lh875spzsl6rnc4058grxvn-autoconf-2.71.tar.xz.drv'...
machine # error: 1 dependencies of derivation '/nix/store/5b0mvjy9yqhphn6jf7hz2lrxqrdcfgb9-libarchive-3.6.1.drv' failed to build

Suggestion 2: stdenv -> stdenvNoCC

machine # warning: error: unable to download 'http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz': Couldn't resolve host name (6); retrying in 337 ms
machine # warning: error: unable to download 'http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz': Couldn't resolve host name (6); retrying in 648 ms
machine # warning: error: unable to download 'http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz': Couldn't resolve host name (6); retrying in 1407 ms
machine # [   37.506805] systemd[1]: Started Logrotate Service.
machine # [   37.531086] systemd[1]: logrotate.service: Deactivated successfully.
machine # warning: error: unable to download 'http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz': Couldn't resolve host name (6); retrying in 2556 ms
machine # error: unable to download 'http://tarballs.nixos.org/stdenv-linux/x86_64/c5aabb0d603e2c1ea05f5a93b3be82437f5ebf31/bootstrap-tools.tar.xz': Couldn't resolve host name (6)
machine # error: builder for '/nix/store/bzq60ip2z5xgi7jk6jgdw8cngfiwjrcm-bootstrap-tools.tar.xz.drv' failed with exit code 1

@thomasjm
Copy link
Contributor Author

thomasjm commented Feb 6, 2023

I just rebased everything on master and force pushed, including your two suggestions @roberth. So on the current branch the tests fail on HEAD and HEAD~1, but pass on HEAD~2.

@roberth
Copy link
Member

roberth commented Feb 7, 2023

Ok, back from a deep rabbit hole that included improving the NixOS/nix testing infrastructure.
Anyway, both errors are caused by dependencies that need to be included into the VM's closure, so that they don't have to be built - well, attempted to be built, and fetched.
A way to achieve that is with inputDerivation, which didn't quite work yet. #215173
If you could squash the commits after removing my broken suggestions, I think we can move this forward. You can omit the co-authored-by stuff, because this is 99.9% your work anyway.

@github-actions github-actions bot removed 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: python labels Feb 8, 2023
@thomasjm
Copy link
Contributor Author

thomasjm commented Feb 8, 2023

Okay, squashed and rebased again. Sorry for the noise all.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 8, 2023
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs look very good! As always, only cosmetic comments. :)


Nix packages are most commonly shared between machines using [HTTP, SSH, or S3](https://nixos.org/manual/nix/stable/package-management/sharing-packages.html), but a flat-file binary cache can still be useful in some situations. For example, you can copy it directly to another machine, or make it available on a network file system. It can also be a convenient way to make some Nix packages available inside a container via bind-mounting.

Note that this function is meant for advanced use-cases. The more idiomatic way to work with flat-file binary caches is via the [nix-copy-closure](https://nixos.org/manual/nix/stable/command-ref/nix-copy-closure.html) command. You may also want to consider [dockerTools](#sec-pkgs-dockerTools) for your containerization needs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Note that this function is meant for advanced use-cases. The more idiomatic way to work with flat-file binary caches is via the [nix-copy-closure](https://nixos.org/manual/nix/stable/command-ref/nix-copy-closure.html) command. You may also want to consider [dockerTools](#sec-pkgs-dockerTools) for your containerization needs.
Note that this function is meant for advanced use-cases. The more idiomatic way to work with flat-file binary caches is via the [`nix-copy-closure`](https://nixos.org/manual/nix/stable/command-ref/nix-copy-closure.html) command. You may also want to consider [`dockerTools`](#sec-pkgs-dockerTools) for your containerization needs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually looks worse when rendered because the links no longer render as blue, so you can't easily tell it's a link.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a problem to be solved in CSS. The source should have rich semantics regardless of the representation du jour. Making things inline code tells the reader it's a code token and not some weird jargon term or other proper name.

doc/builders/images/binarycache.section.md Show resolved Hide resolved
doc/builders/images/binarycache.section.md Show resolved Hide resolved
doc/builders/images/binarycache.section.md Show resolved Hide resolved
@thomasjm
Copy link
Contributor Author

thomasjm commented Feb 8, 2023

Has the time come at last @roberth? 😃

I know we discussed using the new self-contained outputs thing here, but at this point the PR has been open for so long I'd love to get it merged and pursue future improvements in a subsequent PR.

@roberth roberth merged commit 1991c40 into NixOS:master Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants